Surfaces DataFusion's per-query pool_peak_bytes in the result comment, for both sides.

apache/datafusion#23985 (merged 2026-07-30) added PeakRecordingPool, a delegating MemoryPool wrapper that records the high-water mark of MemoryPool::reserved(), and gave each BenchQuery in the results JSON a pool_peak_bytes field. Nothing reads it yet: the DataFusion path here delegates per-query comparison to ./bench.sh compare_detail, and compare.py's QueryResult parses elapsed and row_count and nothing else. The field reaches the results file and stops there.

So this reads results/<side>/*.json itself, in a new runner/pool_peak.rs. DataFusion keeps the measurement; this repo keeps the presentation.

Why a separate section rather than extra columns

That's forced rather than chosen. The existing per-query table is compare.py output, arriving here as opaque text inside a code fence — adding columns to it means patching DataFusion, which is the boundary this change is deliberately staying on the near side of.

The comment gains a Memory Pool Peaks <details> block between the wall-time report and Resource Usage, holding both the per-query table and the RSS pairing. Those stay together because the pairing is a max over the per-query numbers directly above it, and is hard to read without them.

Why the RSS pairing is per benchmark, not per query

pool_peak_bytes is scoped to one query; monitor.rs samples the process subtree across a whole benchmark invocation. A per-query ratio against a run-wide RSS would not mean anything, so the only pairing offered is per invocation: the largest per-query peak in a run against that run's peak RSS.

Even that is labelled conservatively. Both numbers are high-water marks over the same window, but they need not occur at the same instant, and RSS additionally covers data loading and allocator retention. The section says the gap is an upper bound on what the pool did not account for, not a measurement of it.

Measurement only

No thresholds, no gates, no CI check on the ratio. apache/datafusion#22626 and its fix #22742 were reverted wholesale by #22860 for asserting a relationship between pool accounting and real allocation that DataFusion does not claim — operators deliberately leave in-flight batches untracked. #23985 got in by being measurement only; this keeps that property.

Degrading when the field isn't there

Two ways it can be absent, indistinguishable from the JSON alone, so the note names both:

• No memory limit. DataFusion installs the recorder only alongside a pool it has a limit for. That's the default for runs here, so the field is normally absent — you opt in with DATAFUSION_RUNTIME_MEMORY_LIMIT in the trigger comment, per-side or shared.
• A side predating #23985, e.g. baseline: ref: v45.0.0 against main — a normal request.

Missing renders as n/a with a note naming the side, never as zero. A recorded 0 renders as 0 B and is never conflated with missing. When neither side recorded anything the section is omitted entirely, so a default run posts the comment it posted before.

Attribution

Results file names don't follow benchmark names (tpch writes tpch_sf1.json, topk_tpch writes run_topk_tpch.json, h2o writes one per run type), so there's nothing to match on. Each invocation is bracketed with a directory snapshot and whatever JSON appears is attributed to it.

Criterion targets produce no results JSON and are absent from the section.

Testing

14 new tests, biased toward the awkward cases rather than the happy path: field absent on one side, absent on both, present with a zero value, a query in one side's JSON but not the other, one benchmark writing several results files, unparseable and non-JSON files in the results directory, missing resource stats, and RSS sampling in under the pool peak.

cargo fmt, cargo build, cargo clippy --all-targets --all-features -- -D warnings all clean; 142 tests pass.

Beyond the unit tests, dfbench was built at #23985's merge commit (f398301) and both cases were run for real, then put through collect_new → format_pool_peak_section — the same code path the runner uses:

• dfbench nlj --query 1 -i 2 --memory-limit 512M emits "pool_peak_bytes": 262528, which parses to Some(262528) and renders as 256.4 KiB.
• The same command without --memory-limit emits query keys ['query', 'iterations', 'start_time', 'success'] — no pool_peak_bytes — which parses to None and renders as n/a with the note naming the side.

The renderer was also exercised against a real saved clickbench_partitioned.json (43 queries, pre-#23985 shape, "Query 0"-style string ids).

What was not verified: the full runner path — /workspace clones, K8s, and GitHub — was not exercised, so the snapshot/attribution logic has only been tested against synthetic results directories rather than a live bench.sh run. A real end-to-end run is worth doing before relying on this.

Follow-on, not in this change

A companion alloc_peak (peak live heap bytes) is specified in the original handoff via a tracking GlobalAlloc. That predates a cheaper option — arrow-rs's TrackingMemoryPool plus claim() through ArrowMemoryPool, which is shared-buffer-accurate where a GlobalAlloc counter is not. As of 2026-07-29 no operator calls claim() and apache/datafusion#22898 has no picked direction, so the allocator route isn't obsolete, just no longer the default. Worth evaluating both once this has produced real numbers.

🤖 Generated with Claude Code